Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

benchmark: fix fs\bench-realpathSync.js #11904

Closed
wants to merge 1 commit into from
Closed

benchmark: fix fs\bench-realpathSync.js #11904

wants to merge 1 commit into from

Conversation

vsemozhetbyt
Copy link
Contributor

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

benchmark, fs

fs\bench-realpathSync.js runs safely only if its cwd is its __dirname. In other cases, it throws ENOENT: no such file or directory. This commit makes it callable from any place (including being a part of the fs suite launched from any directory).

@vsemozhetbyt
benchmark: fix fs\bench-realpathSync.js
b02d002 
Make it call-site-cwd-independent.
@nodejs-github-bot nodejs-github-bot added the benchmark Issues and PRs related to the benchmark subsystem. label Mar 17, 2017
@mscdex
Copy link
Contributor

mscdex commented Mar 17, 2017

I've been running that benchmark quite a bit lately and haven't had problems running it outside of the benchmark/fs/ directory, including via benchmark/compare.js. How are you triggering the error exactly?

@mscdex mscdex added the fs Issues and PRs related to the fs subsystem / file system. label Mar 17, 2017
@vsemozhetbyt
Copy link
Contributor Author

@mscdex

J:\temp\_git\node-fork\benchmark\fs>bench-realpathSync.js
fs\bench-realpathSync.js type="relative" n=10000: 4,399.895823842575
fs\bench-realpathSync.js type="resolved" n=10000: 4,541.6802584585585

J:\temp>j:\temp\_git\node-fork\benchmark\fs\bench-realpathSync.js
fs.js:960
  binding.lstat(pathModule._makeLong(path), statValues);
          ^

Error: ENOENT: no such file or directory, lstat 'J:\lib'
    at Object.fs.lstatSync (fs.js:960:11)
    at Object.realpathSync (fs.js:1630:21)
    at relativePath (J:\temp\_git\node-fork\benchmark\fs\bench-realpathSync.js:31:8)
    at main (J:\temp\_git\node-fork\benchmark\fs\bench-realpathSync.js:21:5)
    at Benchmark.process.nextTick (J:\temp\_git\node-fork\benchmark\common.js:34:28)
    at _combinedTickCallback (internal/process/next_tick.js:73:7)
    at process._tickCallback (internal/process/next_tick.js:104:9)
    at Module.runMain (module.js:607:11)
    at run (bootstrap_node.js:425:7)
    at startup (bootstrap_node.js:146:9)

@vsemozhetbyt
Copy link
Contributor Author

Also:

J:\temp>node _git\node-fork\benchmark\compare.js --old node.v8-5.9.exe --new node.v8-5.9.turbo-on.exe --filter bench-realpathSync.js fs > bm.csv
[00:00:01|%   0| 0/1 files |  0/60 runs | 0/2 configs]: fs\bench-realpathSync.jsfs.js:878
  binding.lstat(pathModule._makeLong(path), statValues);
          ^

Error: ENOENT: no such file or directory, lstat 'J:\lib'
    at Object.fs.lstatSync (fs.js:878:11)
    at Object.realpathSync (fs.js:1548:21)
    at relativePath (J:\temp\_git\node-fork\benchmark\fs\bench-realpathSync.js:31:8)
    at main (J:\temp\_git\node-fork\benchmark\fs\bench-realpathSync.js:21:5)
    at Benchmark.process.nextTick (J:\temp\_git\node-fork\benchmark\common.js:34:28)
    at _combinedTickCallback (internal/process/next_tick.js:73:7)
    at process._tickCallback (internal/process/next_tick.js:104:9)
    at Module.runMain (module.js:607:11)
    at run (bootstrap_node.js:425:7)
    at startup (bootstrap_node.js:146:9)

@vsemozhetbyt vsemozhetbyt mentioned this pull request Mar 19, 2017
Closed
3 tasks
@vsemozhetbyt
Copy link
Contributor Author

Linter CI: https://ci.nodejs.org/job/node-test-linter/7706/

@vsemozhetbyt
Copy link
Contributor Author

@mscdex Is it OK to land this or should I provide more info?

@mscdex
Copy link
Contributor

mscdex commented Mar 21, 2017

It's fine by me I guess.

@vsemozhetbyt
Copy link
Contributor Author

Landed in ab2d49b

vsemozhetbyt added a commit that referenced this pull request Mar 21, 2017
@vsemozhetbyt
benchmark: fix fs\bench-realpathSync.js
ab2d49b 
Make it call-site-cwd-independent.

PR-URL: #11904
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@vsemozhetbyt vsemozhetbyt deleted the bench-realpathSync branch March 21, 2017 12:07
jungx098 pushed a commit to jungx098/node that referenced this pull request Mar 21, 2017
@vsemozhetbyt @jungx098
benchmark: fix fs\bench-realpathSync.js
ce24d52 
Make it call-site-cwd-independent.

PR-URL: nodejs#11904
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@seishun
Copy link
Contributor

seishun commented Mar 21, 2017

Not a big thing, but I'd just like to note for future reference that forward slashes are preferable in paths.

@vsemozhetbyt
Copy link
Contributor Author

@seishun Thank you. Sorry.

MylesBorins pushed a commit that referenced this pull request Mar 28, 2017
@vsemozhetbyt @MylesBorins
benchmark: fix fs\bench-realpathSync.js
d62ddbe 
Make it call-site-cwd-independent.

PR-URL: #11904
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@MylesBorins MylesBorins mentioned this pull request Mar 28, 2017
Merged
@italoacasas italoacasas mentioned this pull request Apr 10, 2017
Merged
2 tasks
@MylesBorins
Copy link
Contributor

MylesBorins commented Apr 18, 2017 •
edited
Loading

This does not land cleanly in LTS. Please feel free to manually backport. Please also feel free to replace do-not-land if it is being backported

edit: nvm it lands 🎉

MylesBorins pushed a commit that referenced this pull request Apr 18, 2017
@vsemozhetbyt @MylesBorins
benchmark: fix fs\bench-realpathSync.js
57c8dde 
Make it call-site-cwd-independent.

PR-URL: #11904
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins pushed a commit that referenced this pull request Apr 19, 2017
@vsemozhetbyt @MylesBorins
benchmark: fix fs\bench-realpathSync.js
abbf6e3 
Make it call-site-cwd-independent.

PR-URL: #11904
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@MylesBorins MylesBorins mentioned this pull request Apr 19, 2017
Merged
andrew749 pushed a commit to michielbaird/node that referenced this pull request Jul 19, 2017
@vsemozhetbyt @andrew749
benchmark: fix fs\bench-realpathSync.js
0f157c1 
Make it call-site-cwd-independent.

PR-URL: nodejs/node#11904
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@addaleax addaleax addaleax approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
benchmark Issues and PRs related to the benchmark subsystem. fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@vsemozhetbyt @mscdex @seishun @MylesBorins @jasnell @addaleax @nodejs-github-bot